Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: electron:serve起動時にエディタを起動しないオプションを追加 #2309

Merged
merged 5 commits into from
Oct 28, 2024

Conversation

sabonerune
Copy link
Contributor

内容

いつからかはっきり覚えていないのですがVSCodeを使用したメインプロセスのデバッグの起動が自分の環境では非常に遅くなりました。(自分のPCのスペックが低いというのもありますが…)
恐らくデバッガが開発サーバーを監視していることが原因だと思います。

このPRで開発サーバーの起動とElectronの起動を分離することでメインプロセスデバッグ時の起動を高速化します。

その他

SKIP_LAUNCH_EDITOR環境変数に1を設定したうえでelectron:serveを実行するとエディタの起動とメインプロセスのリロードが行われなくなります。
この状態で npx electron . --no-sandboxを実行するとエディタが起動します。
そのためのlaunch.template.jsontasks.template.json追加しました。

VITE_DEV_SERVER_URLはビルド時に埋め込まれます。

レンダラープロセスのsourcemapが正しく検出できていなかったので変更しました。

@sabonerune sabonerune requested a review from a team as a code owner October 22, 2024 09:15
@sabonerune sabonerune requested review from Hiroshiba and removed request for a team October 22, 2024 09:15
@Hiroshiba
Copy link
Member

Hiroshiba commented Oct 22, 2024

起動が高速になるオプションを用意するのは良さそうに思いました!!
ただ起動方法が増えるといつの間にかデグレッてしまう可能性もあると思うので、ドキュメントで軽く案内しときたい気持ちが。

現状ちょっとわかってないので質問させてください 🙇

Launch Electron Main/Renderer without electron:serveを実行すると速い、という認識で合ってそうでしょうか?
これは何で速いんでしょう・・・?

というのも、僕の理解だとこれを実行すると↓の2つが実行されて、
Attach to Renderer Process
Launch Electron without electron:serve
2つ目の方はさらにpreLaunchTaskで
SKIP_LAUNCH_EDITOR=1 npm run electron:serve
が実行されるという理解です。

ここでまずwithout electron:serveという名前なのにelectron:serveが実行されてるのが違和感ありました。
(つまり、僕の認識が間違ってそうな気がしてます)

で、SKIP_LAUNCH_EDITORを指定すると名前から察するに「エディターが起動しない」だと思うのですが、さらにLaunch Electron without electron:serve内で
electron . --no-sandbox
が実行されていて、これは @sabonerune さんの仰るエディタが起動するだと思うので、利点を消してるように見えるんですよね。

つまり今mainブランチで実装されているものと、今回プルリクエストで追加されたものが全く同じ挙動するような気がしてて、なぜ速くなるのかわかってない感じです。

(たぶん僕が開発サーバーの起動とElectronの起動を分離するというのがなんなのかを理解できてないんだと思ってます。。知識が不足しててすみません。。 🙇 )

@sabonerune
Copy link
Contributor Author

@Hiroshiba
そもそもの起動が遅い原因はelectron:serveをF5キーで起動するとVSCodeのデバッガがViteの動作(トランスコンパイルやeslint等の実行)をトレースしてしまうことが原因だと考えています。

これを回避するためデバッガを通さずにelectron:serveを起動してElectronのみをデバッガで起動できるようにするというのがこのPRの意図です。

追加したLaunch Electron Main/Renderer without electron:serveはこの操作を自動化するためのものです。
(preLaunchTaskで起動したelectron:serveはデバッグの対象にならずElectronを終了しても起動し続ける)

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

申し訳ないです、ちょっと正直判断つかなくてどうすればいいのかが分からないでいます 🙇
デバッガを使いやすくするのは良さそうだと思いつつ、ちょっと名称周りで複雑性が増していると思うのでそこ整理したいです。

あと、もし環境変数を作らずに実現できるんだったら、かなり嬉しいかなと。
(なぜそれがあるのかを説明を書かないといけないので)
逆に言うと、もし環境変数がそのままでもドキュメントや名称がしっかりしてれば別に良さそう!

一旦語句の整理をしたいです!

SKIP_LAUNCH_EDITOR

エディターとは何を指してますか? 👀 electron・・・?
雰囲気見てるとelectronのstartupが呼ばれなそうなので、起動しないものはエディタ(UI?)ではなくelectronなのかなぁみたいな。

でもelectronが起動しないなら、これは一体何をしてるんだろう・・・・・?
viteのserve・・・?

Electron Serve Only

electron serveがまた何かわからないかもです。
npm run electron:serve自体がelectron serveっぽくて、npm run electron:serveSKIP_LAUNCH_EDITOR=1で起動することをElectron Serve Onlyと呼ぶとかなりややこしいなぁと。

まあこれはさっきのフラグ名もそうですが、何をしているのか一旦整理できると嬉しいそう。
雰囲気、electron serve = vite serve + launch electronかも?
vite serveというのが理解あってるかかなり自信ないです。。。)

Launch Electron Main/Renderer without electron:serve

without electron:serveなのにnpm run electron:serveをするので、かなり分かりづらいはず。
これの名称はまあ最後に考えればよさそう。

というより、そもそもvite serveの方をデバッグしたい事ってあるんでしょうか 👀
SKIP_LAUNCH_EDITOR=1だとレンダラープロセスのデバッガが動かない感じでしたっけ。
や、片方だけにできないかな~と思った次第でした。多分無理そう。

この辺の語句を整理してから、やりたいことベースに名前をつけるか、やることベースで名前をつけるかしていきたみです!
この辺り適当にやると迷宮化するので、申し訳ないんですが丁寧に作って行きたく 🙇
一番知識があるのは @sabonerune さんだと思うので、ちょっとお聞きすることが多いかもしれませんが、もしよければ 🙇 🙇


@yamachu 突然すみません!!

レンダラープロセスのデバッガを起動せず、メインプロセスだけデバッガを働かせる良い方法とかってあったりしませんかね 👀

@sabonerune
Copy link
Contributor Author

sabonerune commented Oct 24, 2024

あと、もし環境変数を作らずに実現できるんだったら、かなり嬉しいかなと。

npm run electron:…で起動できるようにするということでしょうか?
最初は考えたのですが適切な名前が思い浮かばなかったのと需要があまりなさそうなので増やすのもよくないなと思った感じです。
(現状のelectron:serveelectron:launchにするのが合っていそうな気がするのですが影響が大きすぎるので…)

エディターとは何を指してますか? 👀 electron・・・?
雰囲気見てるとelectronのstartupが呼ばれなそうなので、起動しないものはエディタ(UI?)ではなくelectronなのかなぁみたいな。

Electronのことであっています。

electron serveがまた何かわからないかもです。

まず現状のelectron:serveがやっていることをあげると

  • レンダラープロセスをトランスコンパイルしてVITE_DEV_SERVER_URLで配信
  • メインプロセスをビルドしてdist/に書き出す
  • ファイル変更の検出と再コンパイル
  • vite-plugin-checker(eslintとtsc)の実行
  • electronの実行・再起動

という感じです。
serveは開発サーバー起動のことだよなと考えてこのような名前にしました。

というより、そもそもvite serveの方をデバッグしたい事ってあるんでしょうか 👀

多分ないと思います。
レンダラープロセスのデバッグはElectronのDevToolやAttach to Renderer Processで今まで通り行います。

@Hiroshiba
Copy link
Member

あと、もし環境変数を作らずに実現できるんだったら、かなり嬉しいかなと。

npm run electron:…で起動できるようにするということでしょうか?

あっすみません!
vite.config.mts内でSKIP_LAUNCH_EDITORを定義しなくてもできるようになると、という意図でした!
どちらかというと「vite.config.mtsの変更がほぼなしで」という意図が近いかもです。まあ、無理そう!

serveは開発サーバー起動のことだよなと考えてこのような名前にしました。

なるほどです!!
うーーーーん。。。。
まーーーとりあえずElectron Serveというと、electron:serveを指しちゃう(ので意図しなければ使わない)前提に考える感じでいく感じで。。。!!

というより、そもそもvite serveの方をデバッグしたい事ってあるんでしょうか 👀

多分ないと思います。
レンダラープロセスのデバッグはElectronのDevToolやAttach to Renderer Processで今まで通り行います。

おおなるほど!!
そうか、Attach to Renderer Processはあるから普通にレンダラー側もデバッグできるんですね!!

じゃあ、launch.jsonに元から実装されていたものの完全上位互換、のはずって感じで合ってそうでしょうか。
だとしたら、勇気を持って既存のを置き換える形で実装がいい気がしますね!!

一旦この方向で、あとは名称整理してマージを目指せればと・・・!!!


ちょっと名称整理一部こんな感じでってのを列挙してみました!
認識違ってたら遠慮なく言ってください、もうこの辺りは僕より @sabonerune さんのが詳しいと思うので。。。

  • electron:serve → 環境変数なしのnpm run electron:serveのこと
  • Electron Serve → electron:serveのこと
  • Editor → 何を指すか一旦不明ということで。今回の文脈だとElectronに置き換えれる?

書いてみたけど候補先がほとんど書いてないですね。。。

@yamachu
Copy link
Member

yamachu commented Oct 27, 2024

遅くなりました 🙇

MainProcessのデバッグですが、既に @sabonerune さんが追加してくださった

https://github.com/VOICEVOX/voicevox/pull/2309/files#diff-97f96c9eb16d298a5fd0cd4cce5b94915768381efd5d8f313254a65c99763402R34

これで実現可能なので、特に何かを追加することは不要ですね!
VSCodeから確認できるcall stackもスッキリしました、ありがとうございます!

@sabonerune
Copy link
Contributor Author

じゃあ、launch.jsonに元から実装されていたものの完全上位互換、のはずって感じで合ってそうでしょうか。

メインプロセスとプレロードスクリプトの自動リロードが機能しなくなります。
変更したら手動でElectronを終了して再度立ち上げる必要があります。

@Hiroshiba
Copy link
Member

Hiroshiba commented Oct 27, 2024

@yamachu 確認ありがとうございます!!

@sabonerune
あーなるほどです!!
では・・・今までのはそのままに、新しい方をLaunch Electron Main/Renderer without auto reloadとかにしますか!!

auto reloadのとこはhot reloadでもなんでもという感じです!
それで、実際に何をしているのか1行コメントを足してあげると良さそう。


名称に関していろいろコメントしましたが、難しそうであればこちらでよしなに決めてしまおうと思います!
ただ考えるのにも時間が掛かるし不正確になりがちなので、一旦コメントを元にいろんな箇所の名前を変えていただけると助かります 🙇
それを元に変更させていただこうかなと!!

お手数おかけしてしまってすみません。。。。。 🙇

@sabonerune
Copy link
Contributor Author

一通り名前を変更しました。

vite.config.mts Outdated
@@ -45,6 +45,8 @@ export default defineConfig((options) => {
const sourcemap: BuildOptions["sourcemap"] = shouldEmitSourcemap
? "inline"
: false;
const launchEditor =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

変数名はskipLahnchElectronにして、特殊な挙動の目的の方をわかりやすくしてみます。
あと目的コメントも追加します!

Comment on lines 2 to 3
// See https://go.microsoft.com/fwlink/?LinkId=733558
// for the documentation about the tasks.json format
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あっても読まないので消しちゃいますか〜

"tasks": [
{
"label": "Electron Serve without Launch Electron",
// Electronを起動せずにバックグラウンドで"electron:serve"を実行する
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

目的コメントをたさせていただきます 🙏

"stopAll": true
},
{
"name": "Launch Electron Main/Renderer without hot reload",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こっちも目的コメント書き足します 🙏

@Hiroshiba
Copy link
Member

マージします!

デバッグ起動便利ですね!僕もたまに使うかも。
まあ本当はテスト駆動開発するのが良さそうですが、やっぱり現実は難しそう。

@Hiroshiba Hiroshiba merged commit 5a81c98 into VOICEVOX:main Oct 28, 2024
8 checks passed
@sabonerune sabonerune deleted the feat/skip-launch-editor branch October 29, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants